[Win32] Return unrounded value in FontMetrics.getAverageCharacterWidth#2496
[Win32] Return unrounded value in FontMetrics.getAverageCharacterWidth#2496HeikoKlare merged 1 commit intoeclipse-platform:masterfrom
Conversation
HeikoKlare
left a comment
HeikoKlare
left a comment
There was a problem hiding this comment.
Thank you for the proposal! Since FontMetrics is a kind of central class (thus adapting it is critical) some more information about potential impacts (such as on the various consumers in the Eclipse Platform) and how far they were or need to be tested would be good. Also some information on how to test and reproduce changes in behavior would be nice.
@ptziegler fyi, as Draw2d's TextUtilities make use of this method
| */ | ||
| public double getAverageCharacterWidth() { | ||
| return getAverageCharWidth(); | ||
| return DPIUtil.pixelToPoint((float)handle.tmAveCharWidth, getZoom()); |
There was a problem hiding this comment.
Shouldn't we add a method converting doubles instead of trimming to float and then converting back?
HeikoKlare
force-pushed
the
fix-2461
branch
4 times, most recently
from
04870b3 to
5b1bea5
Compare
HeikoKlare
left a comment
HeikoKlare
left a comment
There was a problem hiding this comment.
The current implementation of FontMetrics#getAverageCharacterWidth() obviously returns imprecise results when not at 100% zoom. The method has been introduced to deal with character widths not being integers on MacOS without adapting the Win32 implementation to also return precise autoscaled values: 0aa46cf
This fix resolves the mentioned issue and since we are very early in the development cycle, we should merge this to have several weeks of implicit testing for any potential regression that we may not anticipate now.
The value for getAverageCharacterWidth was rounded, even though its method signature returns a double. The unrounded value provides a better estimation of the average character width for non-integer zoom factors. Fixes: eclipse-platform#2461
2 participants
Fixes: #2461